feat!: #697 added tool input arguments validation#873
feat!: #697 added tool input arguments validation#873ashakirin wants to merge 2 commits intomodelcontextprotocol:mainfrom
Conversation
… causes tool execution error. Breaking change, because validation is activated by default
Kehrlann
added
area/server
P1
Kehrlann
left a comment
Kehrlann
left a comment
There was a problem hiding this comment.
Now that I'm revisiting this, I'm wondering whether we really want this system property to toggle this on or off.
We're going to ship this in 2.0, let it be a breaking change - after all, servers MUST validate tool inputs.
(And we should probably revisit tool name validation too, then)
| @@ -0,0 +1,74 @@ | |||
| /* | |||
| * Copyright 2024-2024 the original author or authors. | |||
There was a problem hiding this comment.
| * Copyright 2024-2024 the original author or authors. | |
| * Copyright 2026-2026 the original author or authors. |
| @@ -0,0 +1,270 @@ | |||
| /* | |||
| * Copyright 2024-2024 the original author or authors. | |||
There was a problem hiding this comment.
| * Copyright 2024-2024 the original author or authors. | |
| * Copyright 2026-2026 the original author or authors. |
| * errors are returned as Tool Execution Errors (isError=true) rather than Protocol | ||
| * Errors, per MCP specification. | ||
| * | ||
| * @author Alireza Khoram |
There was a problem hiding this comment.
Is this author tag intended?
| @@ -0,0 +1,270 @@ | |||
| /* | |||
| * Copyright 2024-2024 the original author or authors. | |||
There was a problem hiding this comment.
| * Copyright 2024-2024 the original author or authors. | |
| * Copyright 2026-2026 the original author or authors. |
| } | ||
| Map<String, Object> inputSchema = jsonMapper.convertValue(tool.inputSchema(), | ||
| new TypeRef<Map<String, Object>>() { | ||
| }); |
There was a problem hiding this comment.
I wonder if we should leave this marshalling on the hot path. Look at what we've done in DefaultJsonSchemaValidator, there's a schema cache to avoid specifically doing to much of these operations.
Maybe we should have something similar here? Or maybe we should update the JsonSchemaValidator to also validate JsonSchema objects?
2 participants
Added tool input arguments validation causes Tool Execution Error accordingly SEP-1303.
It is breaking change, because tool input validation is activated by default